refactor(cli): explicit connectorFromFile, drop ./connectors scan; flatten examples#1043
Merged
Conversation
…atten examples
Connectors are declared explicitly via defineConfig({ connectors:
[connectorFromFile("./x.connector.ts")] }), replacing the ./connectors directory
auto-discovery. Only listed connectors compile + ship; the compile / install /
key-resolution flow is unchanged (the loader produces the same
DesiredConnectorDefinition shape it used to). Connections still reference
connectors by key or defineConnector class.
Flatten the examples for legibility: connector and reaction files move to the
project root next to lobu.config.ts; the vestigial models/reactions/ and
connectors/ directories are gone (reactions were already path-referenced, so
this just colocates them). Updates the docs, the landing snippet generator, and
the lobu starter skill to match.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (9)
📝 WalkthroughWalkthroughThis PR migrates connector and watcher reaction file handling from implicit directory auto-discovery ( ChangesConnector file-path refactoring: from auto-discovery to explicit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Member
Author
|
bug_free 76, simplicity 82, slop 12, bugs 1, 0 blockers typecheck/unit passed; [env] integration refused DATABASE_URL database "postgres" as unsafe, unrelated to diff. Explored by loading desired state for examples/ecommerce, lobu-crm, and agent-community via bun import; connectorFromFile and reaction paths resolved. Suggested fixes
Full verdict JSON{
"bug_free_confidence": 76,
"bugs": 1,
"slop": 12,
"simplicity": 82,
"blockers": [],
"change_type": "refactor",
"behavior_change_risk": "medium",
"tests_adequate": false,
"suggested_fixes": [
{
"file": "packages/cli/src/commands/init.ts",
"line": 215,
"change": "Replace the generated tsconfig includes for `connectors/**/*.ts` and `reactions/**/*.ts` with patterns that cover explicit root files, e.g. `**/*.connector.ts` and `**/*.reaction.ts`."
},
{
"file": "examples/lobu-crm/tsconfig.json",
"line": 13,
"change": "Update this and the other flattened example tsconfigs to include the new root `*.connector.ts` / `*.reaction.ts` files so example typechecks cover them."
},
{
"file": "packages/cli/src/commands/_lib/apply/desired-state.ts",
"line": 217,
"change": "Update the connector comments to remove stale `./connectors` auto-discovery language and delete the duplicate obsolete comment block at lines 709-734."
},
{
"file": "packages/landing/src/components/LandingPage.tsx",
"line": 1001,
"change": "Replace `connectors/` with wording that matches explicit connector files, e.g. `*.connector.ts`."
}
],
"notes": "typecheck/unit passed; [env] integration refused DATABASE_URL database \"postgres\" as unsafe, unrelated to diff. Explored by loading desired state for examples/ecommerce, lobu-crm, and agent-community via bun import; connectorFromFile and reaction paths resolved.",
"categories": {
"src": 126,
"tests": 94,
"docs": 72,
"config": 32,
"deps": 0,
"migrations": 0,
"ci": 0,
"generated": 68
}
}Local review gate — branch protection can require the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/landing/src/content/docs/getting-started/reaction-sdk.md (1)
122-130:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the local file-layout example to root-level reaction paths.
This page still documents
reactions/critical-detection.reaction.tsandreaction: "./reactions/...", which conflicts with the new explicit root-level layout shown elsewhere in this PR.Also applies to: 132-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/landing/src/content/docs/getting-started/reaction-sdk.md` around lines 122 - 130, Update the file-layout example and any inline examples of the reaction path so they show root-level reactions instead of an agent-local reactions folder: move the "reactions/critical-detection.reaction.ts" entry out of the my-agent/ tree to a top-level reactions/critical-detection.reaction.ts in the example and change the config/property example that shows reaction: "./reactions/..." to reference the root-level path used elsewhere in the PR (e.g., "./reactions/critical-detection.reaction.ts"); apply the same replacement for the other occurrences around lines 132–143.
🧹 Nitpick comments (1)
packages/cli/src/commands/_lib/apply/desired-state.ts (1)
709-734: ⚡ Quick winRemove orphaned JSDoc comment describing obsolete auto-discovery behavior.
This comment block documents the removed
discoverLocalConnectorDefinitionsfunction and its./connectors/*.connector.tsauto-discovery behavior. With the function replaced byresolveConnectorSources, this JSDoc is now orphaned (two consecutive JSDoc blocks before a single function). The new comment at lines 735-742 adequately documents the current explicit-list behavior.♻️ Proposed fix
-/** - * Discover local connector definitions for the TypeScript config path. - * - * A `lobu.config.ts` references connectors by key (or via the class returned by - * `defineConnector`); the source the server compiles lives in - * `./connectors/*.connector.ts`. We ship each file's source with `key: null` — - * the server compiles it and resolves the real key, the same contract the YAML - * loader used for auto-discovered `.connector.ts` files. `apply-cmd` then - * compiles each `sourcePath` on the CLI (where the project's node_modules is - * available) and uploads it via `install_connector`. - * - * We intentionally do NOT compile/instantiate the connector here to resolve its - * key eagerly: that would force a full esbuild + module load (and installed - * project deps, and any module-load side effects) on every load — including - * `--dry-run` — for no benefit, since the server is the source of truth for the - * compiled key. The cost is deferred to post-confirmation install in apply-cmd. - * - * Caveat (shared with YAML auto-discovery, see `locallyDeclaredConnectorKeys`): - * because the shipped key is `null`, a connection's config is validated against - * the *fresh* catalog only after install, and a connection that references a - * connector by a bare *string* key relies on that string matching the file's - * compiled `definition.key`. Reference the connector by its `defineConnector` - * class instead (`connector: myConnector`) to make that match exact — the - * mapper resolves the key from `definition.key`, so a typo can't silently bind - * the connection to a different (bundled/remote) connector. - */ /** * Resolve the project's explicit `connectors: [connectorFromFile(...)]` list🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/_lib/apply/desired-state.ts` around lines 709 - 734, The large JSDoc block describing discoverLocalConnectorDefinitions and auto-discovery is now stale/orphaned and precedes the new resolveConnectorSources implementation; remove that obsolete JSDoc so only the current comment describing explicit-list behavior (the new JSDoc that documents resolveConnectorSources) remains directly above the function. Locate the old comment block that references discoverLocalConnectorDefinitions and ./connectors/*.connector.ts and delete it, leaving the new JSDoc (lines mentioning explicit-list behavior and resolveConnectorSources) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/landing/src/content/docs/getting-started/reaction-sdk.md`:
- Line 162: Summary: Replace the em dash used in the landing doc line with a
colon or hyphen. Edit the markdown line containing the link text
"examples/sales/account-health-monitor.reaction.ts" (the sentence that mentions
persisting each one as a typed `health_change` event) and swap the em dash
character (—) for a colon (:) or hyphen (-) so the user-facing copy follows the
guideline against em dashes in landing text.
---
Outside diff comments:
In `@packages/landing/src/content/docs/getting-started/reaction-sdk.md`:
- Around line 122-130: Update the file-layout example and any inline examples of
the reaction path so they show root-level reactions instead of an agent-local
reactions folder: move the "reactions/critical-detection.reaction.ts" entry out
of the my-agent/ tree to a top-level reactions/critical-detection.reaction.ts in
the example and change the config/property example that shows reaction:
"./reactions/..." to reference the root-level path used elsewhere in the PR
(e.g., "./reactions/critical-detection.reaction.ts"); apply the same replacement
for the other occurrences around lines 132–143.
---
Nitpick comments:
In `@packages/cli/src/commands/_lib/apply/desired-state.ts`:
- Around line 709-734: The large JSDoc block describing
discoverLocalConnectorDefinitions and auto-discovery is now stale/orphaned and
precedes the new resolveConnectorSources implementation; remove that obsolete
JSDoc so only the current comment describing explicit-list behavior (the new
JSDoc that documents resolveConnectorSources) remains directly above the
function. Locate the old comment block that references
discoverLocalConnectorDefinitions and ./connectors/*.connector.ts and delete it,
leaving the new JSDoc (lines mentioning explicit-list behavior and
resolveConnectorSources) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27f5046a-053a-450f-b0a6-b31ac94f87dc
⛔ Files ignored due to path filters (1)
packages/landing/src/generated/landing-snippets.jsonis excluded by!**/generated/**
📒 Files selected for processing (39)
examples/agent-community/discourse-posts.connector.tsexamples/agent-community/lobu.config.tsexamples/agent-community/opportunity-matcher.reaction.tsexamples/atlas/catalog-staleness-checker.reaction.tsexamples/atlas/lobu.config.tsexamples/delivery/lobu.config.tsexamples/delivery/shopify-orders.connector.tsexamples/ecommerce/lobu.config.tsexamples/ecommerce/stripe-charges.connector.tsexamples/finance/lobu.config.tsexamples/finance/quickbooks-transactions.connector.tsexamples/finance/reconciliation-monitor.reaction.tsexamples/leadership/linear-cycles.connector.tsexamples/leadership/lobu.config.tsexamples/legal/docusign-envelopes.connector.tsexamples/legal/lobu.config.tsexamples/lobu-crm/README.mdexamples/lobu-crm/funnel-digest.reaction.tsexamples/lobu-crm/funnel-form.connector.tsexamples/lobu-crm/inbound-triage.reaction.tsexamples/lobu-crm/lobu.config.tsexamples/market/exa-news-feed.connector.tsexamples/market/founder-activity-tracker.reaction.tsexamples/market/lobu.config.tsexamples/sales/account-health-monitor.reaction.tsexamples/sales/lobu.config.tsexamples/sales/salesforce-pipeline.connector.tspackages/cli/src/commands/_lib/apply/__tests__/load-config.test.tspackages/cli/src/commands/_lib/apply/desired-state.tspackages/cli/src/config/define.tspackages/landing/scripts/gen-landing-snippets.tspackages/landing/src/components/LandingPage.tsxpackages/landing/src/content/docs/getting-started/connector-sdk.mdpackages/landing/src/content/docs/getting-started/index.mdxpackages/landing/src/content/docs/getting-started/reaction-sdk.mdpackages/landing/src/content/docs/reference/cli.mdpackages/landing/src/content/docs/reference/lobu-config.mdpackages/landing/src/content/docs/reference/reaction-sdk.mdskills/lobu/SKILL.md
| ## See it in production | ||
|
|
||
| - [`examples/sales/models/reactions/account-health-monitor.reaction.ts`](https://github.com/lobu-ai/lobu/blob/main/examples/sales/models/reactions/account-health-monitor.reaction.ts) — filters worsening risk transitions out of a watcher's account-changes extraction and persists each one as a typed `health_change` event. | ||
| - [`examples/sales/account-health-monitor.reaction.ts`](https://github.com/lobu-ai/lobu/blob/main/examples/sales/account-health-monitor.reaction.ts) — filters worsening risk transitions out of a watcher's account-changes extraction and persists each one as a typed `health_change` event. |
There was a problem hiding this comment.
Replace the em dash in landing doc copy.
This line uses an em dash in user-facing text; please switch to a colon or hyphen.
As per coding guidelines, "packages/landing/**/*.{ts,tsx,md}: Do not use em dashes in user-facing text in landing copy".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/landing/src/content/docs/getting-started/reaction-sdk.md` at line
162, Summary: Replace the em dash used in the landing doc line with a colon or
hyphen. Edit the markdown line containing the link text
"examples/sales/account-health-monitor.reaction.ts" (the sentence that mentions
persisting each one as a typed `health_change` event) and swap the em dash
character (—) for a colon (:) or hyphen (-) so the user-facing copy follows the
guideline against em dashes in landing text.
- lobu init + all example tsconfigs: include `**/*.connector.ts` / `**/*.reaction.ts` so the flattened root-level files are covered (was connectors/** + reactions/**). - desired-state.ts: drop the orphaned discoverLocalConnectorDefinitions doc block and the stale "./connectors" auto-discovery wording. - LandingPage.tsx: connectors/ → *.connector.ts in the self-host blurb.
buremba
added a commit
that referenced
this pull request
May 25, 2026
…./connectors scan) (#1048) PR #1043 removed the ./connectors auto-discovery in favor of explicit connectorFromFile(), and swept every examples/*/lobu.config.ts over — but the sdk-e2e gate scaffolds its own hermetic fixture inline in a shell-script heredoc (not in examples/), so the migration missed it. The fixture's local pulse.connector.ts was no longer discovered, so connection "pulse" referenced an uninstalled connector "sdke2e-pulse" → apply halted → main went red. Declare the connector explicitly to match the new API.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Connectors are now declared explicitly, replacing the
./connectorsdirectory auto-discovery:This completes the explicit-over-magic arc: reactions were already path-referenced, skills became explicit (#1039), and connectors were the last auto-scanned directory.
Why
./connectorswas the same invisible discover-and-ship-everything scan we removed for skills. ExplicitconnectorFromFilemeans only referenced connectors compile + upload, the config shows what ships, and connectors can live anywhere — so the examples flatten to a single legible folder (the actual motivation):The vestigial
models/(a leftover of the old file-first layout) andconnectors/dirs are gone across all examples.How it stays low-risk
The connection↔connector key wiring is decoupled from source-shipping:
connectorKey()resolves from the string/class in the pure mapper, exactly as before. This change only swaps how the source list is produced — the loader builds the sameDesiredConnectorDefinition[]({ key: null, sourcePath, sourceCode }) from the explicit list instead of areaddir. The compile / install / catalog / key-resolution flow inapply-cmdis untouched.init-from-orgis unaffected (it never emitted local connector source — it references by key string).Scope
connectorFromFile+Project.connectors(define.ts); scan →resolveConnectorSourceswith path guards mirroringresolveReactionScript(desired-state.ts).models/+connectors/dirs removed.LandingPage.tsxprompt copy, lobu-crm README, and thelobustarter skill. Also corrected a stale "reactions auto-pair by filename" claim (they wire via the explicitreaction:path).Verification
bun run typecheck(strict) clean; 151 CLI unit tests pass (connector tests rewritten: explicit ship,--onlyskip, sort, missing-file + path-escape guards).cd packages/landing && bun run buildclean (86 pages; prebuild regenerates snippets from the new paths).loadDesiredStateFromConfigon all 12 examples loads — 9 connectors resolved fromconnectorFromFile, reactions from the flattened paths.Note: the connector compile+install+run path itself is unchanged, so this is covered by typecheck + unit + the load-level E2E; no behavioral change to how connectors execute.
Summary by CodeRabbit
New Features
Documentation
Refactor